Skip to content

Jcs/20260312#529

Open
jSylvestre wants to merge 2 commits intomasterfrom
JCS/20260312
Open

Jcs/20260312#529
jSylvestre wants to merge 2 commits intomasterfrom
JCS/20260312

Conversation

@jSylvestre
Copy link
Copy Markdown
Member

@jSylvestre jSylvestre commented Mar 12, 2026

Summary by CodeRabbit

  • Refactor
    • Enhanced code organization for the payment invoice action interface, improving maintainability and clarity of the submission, validation, and error handling workflows.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 12, 2026

📝 Walkthrough

Walkthrough

A new React component PayInvoiceAction has been extracted from the PayInvoiceContainer, consolidating payment submission UI logic including currency formatting, validation states, error handling, and button state management into a dedicated reusable component.

Changes

Cohort / File(s) Summary
New Payment Action Component
src/Payments.Mvc/ClientApp/src/components/payInvoiceAction.tsx
New React component that renders payment submission UI with total amount, optional due date, conditional alerts for error/validation/success states, and a submit button with dynamic disabled states and labels.
Container Integration
src/Payments.Mvc/ClientApp/src/containers/PayInvoiceContainer.tsx
Replaced inline payment action UI markup with the new PayInvoiceAction component, removing duplicated validation state rendering and button logic; consolidated conditional rendering for editable vs. non-editable invoice states.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested reviewers

  • srkirkland
  • sprucely

Poem

🐰 A component hops forth, so clean and so bright,
Extracting the UI with all of its might,
The Container now lighter, the Action stands tall,
Reusable and focused—a triumph for all! 🎉

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Jcs/20260312' is vague and non-descriptive, using only a branch name format without conveying the actual change being made to the codebase. Use a descriptive title that summarizes the main change, such as 'Extract PayInvoiceAction component' or 'Refactor payment invoice UI into dedicated component'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch JCS/20260312
📝 Coding Plan for PR comments
  • Generate coding plan

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (3)
src/Payments.Mvc/ClientApp/src/components/payInvoiceAction.tsx (2)

77-77: Inline style alignContent: 'center' may have no effect.

The alignContent CSS property only works on multi-line flex containers or grid containers. Without display: flex on this div, the style does nothing.

♻️ Suggested fix
-        <div style={{ alignContent: 'center' }}>
+        <div style={{ display: 'flex', justifyContent: 'center' }}>

Or better, use a CSS class consistent with the rest of the styling approach.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Payments.Mvc/ClientApp/src/components/payInvoiceAction.tsx` at line 77,
The inline style alignContent: 'center' on the div in the payInvoiceAction
component is ineffective because the div is not a multi-line flex or grid
container; either remove alignContent or make the div a flex/grid container and
use the correct alignment properties (e.g., display: 'flex' with
justifyContent/alignItems) or, preferably, replace the inline style with a CSS
class consistent with the project's styling conventions; update the JSX div (the
one currently using style={{ alignContent: 'center' }}) to use the chosen
approach.

14-106: Consider using a functional component.

This component has no state or lifecycle methods, making it a good candidate for a functional component, which is more concise and aligns with modern React patterns. However, if the codebase convention is to use class components, this is fine as-is.

♻️ Optional: Functional component alternative
import * as React from 'react';
import { formatCurrencyLocale } from '../utils/currency';

interface IProps {
  dueDate: string | null;
  errorMessage: string;
  isSaving: boolean;
  isValid: boolean;
  isValidating: boolean;
  onSubmit: () => void;
  total: number;
}

const PayInvoiceAction: React.FC<IProps> = ({
  dueDate,
  errorMessage,
  isSaving,
  isValid,
  isValidating,
  onSubmit,
  total
}) => {
  return (
    <div className='pay-action'>
      {/* ... same JSX ... */}
    </div>
  );
};

export default PayInvoiceAction;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Payments.Mvc/ClientApp/src/components/payInvoiceAction.tsx` around lines
14 - 106, The PayInvoiceAction class component has no state or lifecycle methods
so convert it to a functional component: replace "export default class
PayInvoiceAction extends React.Component<IProps> { public render() { ... } }"
with "const PayInvoiceAction: React.FC<IProps> = ({ dueDate, errorMessage,
isSaving, isValid, isValidating, onSubmit, total }) => { return ( ... ); }
export default PayInvoiceAction;" — keep the existing JSX exactly as-is, keep
the IProps interface and imports (React, formatCurrencyLocale), remove the
render method and class boilerplate, and ensure button onClick, disabled logic
and all prop names (dueDate, errorMessage, isSaving, isValid, isValidating,
onSubmit, total) remain unchanged.
src/Payments.Mvc/ClientApp/src/containers/PayInvoiceContainer.tsx (1)

148-199: Potential redundant UI when invoice is paid.

When invoice.paid is true, canEdit will be false (status won't be 'Sent'), so both the !canEdit block (lines 150-181) and the invoice.paid block (lines 183-197) render. This shows the total twice and displays both a status badge and the "Invoice Paid" confirmation.

Consider refining the conditions to avoid redundancy:

♻️ Suggested condition refinement
-          {(!canEdit || invoice.paid) && (
+          {!canEdit && (
             <div className='pay-action'>
-              {!canEdit && (
+              {!invoice.paid && (
                 <>
                   <span className='pay-action-total'>
                     {formatCurrencyLocale(invoice.total)}
                   </span>

This would show the status info only for non-editable, non-paid invoices, avoiding duplication with the paid confirmation block.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Payments.Mvc/ClientApp/src/containers/PayInvoiceContainer.tsx` around
lines 148 - 199, The UI currently renders both the !canEdit block and the
invoice.paid block when an invoice is paid, causing duplicated information;
update the rendering conditions so the status/total section (the JSX guarded by
!canEdit) only shows for non-editable AND not-paid invoices (i.e., add
invoice.paid === false to that condition), leaving the invoice.paid block (the
"Invoice Paid" and paidDate display) to render only when invoice.paid is true;
adjust the conditional around the status/total alerts (references: canEdit,
invoice.paid, invoice.status, formatCurrencyLocale) so paid invoices don’t also
show the non-paid status UI.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/Payments.Mvc/ClientApp/src/components/payInvoiceAction.tsx`:
- Line 77: The inline style alignContent: 'center' on the div in the
payInvoiceAction component is ineffective because the div is not a multi-line
flex or grid container; either remove alignContent or make the div a flex/grid
container and use the correct alignment properties (e.g., display: 'flex' with
justifyContent/alignItems) or, preferably, replace the inline style with a CSS
class consistent with the project's styling conventions; update the JSX div (the
one currently using style={{ alignContent: 'center' }}) to use the chosen
approach.
- Around line 14-106: The PayInvoiceAction class component has no state or
lifecycle methods so convert it to a functional component: replace "export
default class PayInvoiceAction extends React.Component<IProps> { public render()
{ ... } }" with "const PayInvoiceAction: React.FC<IProps> = ({ dueDate,
errorMessage, isSaving, isValid, isValidating, onSubmit, total }) => { return (
... ); } export default PayInvoiceAction;" — keep the existing JSX exactly
as-is, keep the IProps interface and imports (React, formatCurrencyLocale),
remove the render method and class boilerplate, and ensure button onClick,
disabled logic and all prop names (dueDate, errorMessage, isSaving, isValid,
isValidating, onSubmit, total) remain unchanged.

In `@src/Payments.Mvc/ClientApp/src/containers/PayInvoiceContainer.tsx`:
- Around line 148-199: The UI currently renders both the !canEdit block and the
invoice.paid block when an invoice is paid, causing duplicated information;
update the rendering conditions so the status/total section (the JSX guarded by
!canEdit) only shows for non-editable AND not-paid invoices (i.e., add
invoice.paid === false to that condition), leaving the invoice.paid block (the
"Invoice Paid" and paidDate display) to render only when invoice.paid is true;
adjust the conditional around the status/total alerts (references: canEdit,
invoice.paid, invoice.status, formatCurrencyLocale) so paid invoices don’t also
show the non-paid status UI.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 843922db-f6f6-461f-8499-def9347c4207

📥 Commits

Reviewing files that changed from the base of the PR and between 7f05bde and 32f1aad.

📒 Files selected for processing (2)
  • src/Payments.Mvc/ClientApp/src/components/payInvoiceAction.tsx
  • src/Payments.Mvc/ClientApp/src/containers/PayInvoiceContainer.tsx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant